Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Analyzer] Using StartsWith instead of IndexOf == 0 #78608

Closed
stephentoub opened this issue Nov 20, 2022 · 5 comments · Fixed by dotnet/roslyn-analyzers#6295
Closed

[Analyzer] Using StartsWith instead of IndexOf == 0 #78608

stephentoub opened this issue Nov 20, 2022 · 5 comments · Fixed by dotnet/roslyn-analyzers#6295
Labels
api-approved API was approved in API review, it can be implemented area-System.Globalization code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer partner-impact This issue impacts a partner who needs to be kept updated
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Nov 20, 2022

We should write an analyzer that flags uses of IndexOf that's comparing the resulting index to 0, e.g.
https://grep.app/search?q=%5C.IndexOf%5C%28.%2A%3F%5C%29%20%3D%3D%200&regexp=true&case=true&filter[lang][0]=C%23
Such use would be much better off with StartsWith, as IndexOf will search the entire string vs StartsWith which will only compare at the beginning.

Performance rules Category
Severity = suggestion

@stephentoub stephentoub added code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer labels Nov 20, 2022
@stephentoub stephentoub added this to the 8.0.0 milestone Nov 20, 2022
@ghost
Copy link

ghost commented Nov 20, 2022

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

We should write an analyzer that flags uses of IndexOf that's comparing the resulting index to 0, e.g.
https://grep.app/search?q=%5C.IndexOf%5C%28.%2A%3F%5C%29%20%3D%3D%200&regexp=true&case=true&filter[lang][0]=C%23
Such use would be much better off with StartsWith, as IndexOf will search the entire string vs StartsWith which will only compare at the beginning.

Author: stephentoub
Assignees: -
Labels:

area-System.Globalization, code-analyzer, code-fixer

Milestone: 8.0.0

@MichalPetryka
Copy link
Contributor

Same with Contains and IndexOf != -1/IndexOf > -1/IndexOf >= 0.

@stephentoub
Copy link
Member Author

Same with Contains and IndexOf != -1/IndexOf > -1/IndexOf >= 0.

That already exists:
https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2249

@stephentoub stephentoub added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 21, 2022
@tarekgh
Copy link
Member

tarekgh commented Nov 21, 2022

CC @Youssef1313

@tarekgh tarekgh added blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 21, 2022
@jeffhandley jeffhandley added the partner-impact This issue impacts a partner who needs to be kept updated label Nov 21, 2022
@bartonjs
Copy link
Member

The analyzer/fixer should identify both a.IndexOf(b) == 0 to a.StartsWith(b) and a.IndexOf(b) != 0 to !a.StartsWith(b). And generally apply for the overloads that take comparison options.

Category:Performance
Severity:Suggestion

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Globalization code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer partner-impact This issue impacts a partner who needs to be kept updated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants